-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Pipeline iteration builder #21
base: master
Are you sure you want to change the base?
Conversation
Make PipelineIteration less dependent on the specifics of the stage it is executing. There are small number of TODOs which would be cleaned up in a subsequent change
This allows for PipelineIteration instances to be constructed in a much more flexible way making it a general use pipeline framework.
@Sanrag, I will give this a full review in a few days (trying to enjoy a post-employment computer detoxification), but I want to leave you with some interim thoughts: import com.google.inject.*;
@Singleton
public class Main {
// Guice creates an implicit Provider<T> for its injectables! How nifty!
private final Provider<Builder> builders;
@Inject
public Main(final Provider<Builder> builders) {
this.builders = builders;
}
private void run() {
final Builder builder = builder();
System.out.println("Got builder " + builder);
final Builder another = builder();
System.out.println("Got another builder " + another);
}
public Builder builder() {
return builders.get();
}
public static void main(final String... unused) {
final Main main = Guice.createInjector(new Module()).getInstance(Main.class);
main.run();
}
// NOOP module to demonstrate that Guice will do much of this for you—for better or for worse—
// automagically.
private static class Module extends AbstractModule {
@Override
protected void configure() {}
}
// Hard to know what happens with the scoping here. Give it a try if you want to restrict one of
// these suckers to a more constrained scope.
private static class Builder {
private final SingletonDependency singleton;
@Inject
public Builder(final SingletonDependency singleton) {
this.singleton = singleton;
}
@Override
public String toString() {
return "Builder{" +
"singleton=" + singleton + " " +
"id=" + Integer.toHexString(System.identityHashCode(this)) +
'}';
}
}
@Singleton
public static class SingletonDependency {
@Override
public String toString() {
return "SingletonDependency{" +
"id=" + Integer.toHexString(System.identityHashCode(this)) +
'}';
}
}
// For your own tests …
public static class ScopedDependency {
}
} I think you can effectively get away with the same behavior without passing around or exposing the injector to parts of the stack that ought not be aware of it. Most well-formed Guice implementations hide the injector from as much of the bowels as possible. The example above yields:
|
@Sanrag, polite ping: Is this code review still alive? :) |
We were having discussions around whether we want to make Groningen a general purpose tuning framework (a.k.a. Tunie) or keep it very specific to JVM Tuning. The discussion stretched and I kind of dropped the ball on this. If the decision is to go for latter, I will just go ahead and cancel the pull request. |
I kind remember the discussion was whether to replace jvm tuning with a freeform pipeline or abstract the pipeline objects in order to allow the current Pipeline structure to be used as well as allowing the introduction of this freeform type. My vote being for the latter since there is a significant amount of work that is done based on the nonBuilder style and the Builder style being pretty difficult to enforce any kind of ordering/enumeration/general “this is how the pipeline will behave”ness on... On Mar 26, 2014, at 8:11 AM, Sanrag Sood [email protected] wrote:
|
This allows for PipelineIteration instances to be constructed in a much more flexible way making it a general use pipeline framework.
This pull request is based on #17 and will go after it. Please only review Sanrag@71dcda9 for this pull request.